…njaapi.h

Addresses review feedback (erroneous formatting) on PR #8314.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…aders

binaryninjaapi.h does not provide the emulator API, so the wide-integer
type it pulls in for the plugin does not belong there. Move the (windows.h
min/max-guarded) intx include into a shared emulator_intx.h included by the
emulator's own core and API roots instead.

Addresses review feedback on PR #8314.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…MIN/-1

DIVS/MODS/MULS_DP/CMP_S*/MINS/MAXS funneled operands through int64_t via
SignExtend(v, sz, 8), which silently truncated any operand wider than 8 bytes
to its low 64 bits, and native signed division of INT_MIN by -1 is undefined
behavior (SIGFPE on x86).

Add wide-signed helpers (IsNegative/SignedLess/SignedDivideOrModulo) that
operate directly on the 512-bit value: comparisons use sign-aware unsigned
compares, and division works on magnitudes then reapplies the sign, so
INT_MIN / -1 wraps to INT_MIN (and % to 0) instead of trapping.

Addresses bdash review comment on PR #8314.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…gned carry

ADD_OVERFLOW returned (result < left), i.e. the unsigned carry-out, but the
operation denotes the signed overflow flag. Compute it as: both operands share
a sign and the result's sign differs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The rotate-through-carry implementations mixed a pre-shifted value with an
un-rotated operand (RLC) and dropped the low bits instead of wrapping them
(RRC), producing wrong results (e.g. 1-byte RLC(0x80, carry=0, count=1) gave
0x01 instead of 0x00). Implement both as a true rotate of the (bits+1)-bit
{carry:value} quantity, and record m_lastArithmetic like ROL/ROR so a later
flag read is not stale.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The basic two-operand arithmetic cases masked operands only when recording the
flag context, while evaluating the operation on the raw operands, and the
double-precision cases mask to sz/2 for unrelated reasons. This was noted as
confusing in review. Mask both operands to sz once at the top of each case and
use those masked values for both the computation and m_lastArithmetic, and add
a comment explaining why the double-precision ops mask differently. No behavior
change.

Addresses bdash review comment on PR #8314.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…dary

ADC/SBB folded the carry-in into the stored right operand as
MaskToSize(right + carry, sz), which wraps to 0 when right is all-ones and
carry is 1, so the carry/borrow flag was then computed against the wrong value
(e.g. SBB(0, 0xFFFFFFFF, 1) reported no borrow though one occurs). Track the
carry-in explicitly in the flag context and compute carry-out/borrow-out from
the full-width sum/difference so it no longer wraps. Half-carry now also
accounts for the carry-in.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…edness

For DIVU_DP/DIVS_DP/MODU_DP/MODS_DP the instruction size is the result (quotient)
size N: the dividend is a double-width 2N value and the divisor is N. The code
instead masked the dividend to N, the divisor to N/2 and the result to N/2,
truncating the high half of any true double-width dividend, and the signed
variants additionally funneled operands through int64_t. Mask the dividend to
2*sz and the divisor/result to sz, and compute the signed variants via a
width-aware helper (also fixing INT_MIN/-1). The *_DP multiplies are unchanged;
a comment now explains why their width convention differs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The unmapped-region skip logic compared the next segment's start against
addr + len (the original request end, which can overflow uint64_t for high
addresses) instead of the running cursor. Compute the gap relative to cur so a
mapped segment following an unmapped hole is not skipped past, and high
addresses do not misbehave.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

EmulatorMemory::Map appended a new segment without checking for overlap with
existing ones. Since FindSegment returns the first containing segment, an
overlapping map left part of the address range shadowed, so reads returned
stale/zero bytes and writes could miss a region. Refuse an overlapping map with
a logged warning, preserving the no-overlap invariant the read/write paths rely
on.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The intrinsic-hook FFI passed output buffers (outValues/outRegs) with no
capacity parameter, and both the C++ and Python bridges wrote one entry per
pair the user hook returned. A hook returning more pairs than the fixed 64-entry
buffers the trampoline allocates overran them (stack corruption). Add a maxCount
parameter to the callback contract and clamp writes to it on both the C++ and
Python sides; the Python side also masks values to their field widths.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The reference-counted base started at 0 and relied on the create helper's
AddAPIRef to reach 1, leaving the object destructible by any transient
AddRef/Release during construction. Start at 1 (the birth reference handed to
the creator) like core's RefCountObject, and have the create helper hand off
that reference without an extra AddRef.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

MapMemory allocates a buffer of a caller-controlled length; a std::bad_alloc /
length_error from a huge or garbage length would propagate out of the extern "C"
map functions, which is undefined behavior. Catch allocation failures in
EmulatorMemory::Map (the single point the four map FFI entry points funnel
through) and turn them into a logged no-op.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

In the CALL/CALL_STACK_ADJUST/TAILCALL cases, a builtin stub or the call hook
can request a stop (e.g. on an error) while returning false, but the code only
checked the boolean return and fell through into the call/tailcall path,
executing further despite the pending stop. Check m_stopReason after the stub
and hook handling and return if a stop was requested.

Addresses bdash review comment on PR #8314.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The Python memory-read hook did result.to_bytes(buf_len, ...), which raises
OverflowError if the hook returns a value that does not fit in buf_len bytes;
the bare except then swallowed it and silently returned False, dropping a valid
hook result. Mask the value to the buffer width first (matching the C++ bridge's
truncating behavior) so it is stored instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The plugin's LLILEmulator wrapper lived in namespace BinaryNinja, which is
reserved for the core API. Move it to its own BinaryNinjaEmulatorAPI namespace,
mirroring the debugger's BinaryNinjaDebuggerAPI.

Addresses plafosse review comment on PR #8314.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…GetView

The view member is reference counted; returning a raw BinaryView* from a public
getter invites use-after-free if the emulator outlives the caller's assumptions.
Return Ref<BinaryView> so callers hold their own reference.

Addresses emesare review comment on PR #8314.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The emulator half-supported a null architecture with scattered guards that
silently no-op'd or fell back to little-endian. Instead reject creation of an
emulator whose view/IL has no architecture (BNCreateLLILEmulator* returns null
with a logged error) and drop the now-unnecessary null-arch guards, so m_arch is
a hard invariant everywhere it is used.

Addresses emesare review comment on PR #8314.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

… convention

SetArgument always used the platform's default calling convention, contradicting
the documented behavior of using the function's own convention. Derive the
calling convention from the function being emulated (via its LLIL function),
falling back to the platform default only when the function has none. Also note
the address-size-as-stack-slot-size assumption for exotic ABIs.

Addresses emesare review comments on PR #8314.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

… path

- docs/guide/emulator.md: the stdout callback example used sys.stdout without
  importing sys; use print(..., end='') so the snippet runs as shown.
- api/python/CMakeLists.txt: the non-BN_API_PATH include fell back to
  ${CMAKE_SOURCE_DIR}/api/cmake/..., which does not exist for an out-of-tree
  build; point it at the api root via a correct relative path (matching warp).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ding

The stdin callback's output buffer was a char* in the FFI, so the generated
Python binding exposed it as c_char_p and ctypes handed the callback an
immutable bytes copy, making the memmove into it write to the wrong memory.
Type the buffer void* across the FFI/C++ bridge so the generated binding uses a
writable c_void_p pointer; the Python callback now receives the real buffer
address to write into.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Replace ~130 raw operand accesses in EvalExpr/ExecuteCurrentInstruction with the
typed LowLevelILInstruction accessors (GetLeftExpr/GetRightExpr/GetCarryExpr,
GetSourceExpr, GetDestRegister, GetHighRegister/GetLowRegister, GetConstant,
GetDestExpr, GetParameterExprs, GetTarget/GetTrueTarget/GetFalseTarget, etc.),
which are self-documenting and avoid the operand-index mixups raw indexing
invites. Each mapping was verified against the LowLevelILInstructionAccessor
specializations to read the same operand index; LLIL_EXTERN_PTR is intentionally
left raw (its constant+offset has no equivalent typed pair). No behavior change;
the full emulator test suite passes.

Addresses bdash review comment on PR #8314.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…on/memory edges

Cover the previously-untested behavior that recent fixes touch: flag computation
(overflow/sign/carry/zero via setcc, signed compare via setl), sign/zero
extension (movsx/movzx), shifts and rotate-through-carry (shl/shr/sar/rcl),
signed division of INT_MIN by -1 (must wrap, not trap), division by zero (stops
with Error), cross-segment memory reads, and rejection of overlapping maps.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

LSL/LSR/ASR masked the shift count with & (sz*8-1), imposing x86-style masking
on every architecture. But the architecture's own count masking is already
encoded in the lifted IL (x86 emits e.g. `al u>> (cl & 0x1f)` and pre-masks
immediates; aarch64 emits a raw `w0 u>> w1`), so re-masking was wrong: it made
8/16-bit x86 shifts by >= the operand width a no-op instead of 0, and imposed
x86 semantics on other arches. Perform the literal shift instead — a count >=
the operand width yields 0 for the logical shifts and a full sign-fill for the
arithmetic shift.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Follow-up to moving intx out of binaryninjaapi.h: the intermediate wrapper header
was misplaced (first under the emulator plugin, which coupled the debugger to it).
Drop it and include the shared api/vendor/intx/intx.hpp directly from the
emulator's own headers, the same copy and style the debugger already uses. No
windows.h min/max guard is needed — the codebase already relies on NOMINMAX in
the adapters that include <windows.h>.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Addresses review feedback on how arguments are placed for functions with
non-default argument locations. SetArgument now consults the emulated
function's parameter variables and places each argument at its actual
location instead of always using the calling convention's defaults:

- register-located parameters are placed in their assigned register
- stack-located parameters are written to the stack slot the function
  reads from (the parameter's storage offset relative to the entry
  stack pointer)

Parameters in any other location fall back to the calling convention's
default placement as before.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Addresses review feedback: MaskToSize, SignExtend, IsNegative,
SignedLess and the signed divide/modulo helpers operate purely on
intx::uint512 values and byte sizes with no LLIL dependency. Move them
from LLILEmulator to the ILEmulator base class so future IL emulators
can reuse them instead of duplicating the logic.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Addresses review feedback on `m_flags[flag] = value ? 1 : 0;`. Every flag
in the architecture model is a single boolean bit — ComputeFlagForRole,
the only producer, always yields 0 or 1, and every consumer treats a flag
as 0/1 — so represent flags as bool. m_flags stores bool, and GetFlag /
SetFlag are bool in / bool out, assigning straight into the map without a
ternary. The FFI boundary keeps its uint8_t representation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Addresses review feedback: the LLIL_CALL and LLIL_CALL_STACK_ADJUST
cases mixed break and return. Since nothing follows the dispatch switch,
break and return are equivalent here; standardize on return to match the
sibling control-flow cases (jumps, tailcalls) and avoid the confusion
that the mixed style could cause.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ntion

Addresses review feedback: SetArgument was updated to use the emulated
function's calling convention, but ReadArgument and WriteReturnValue
still used only the platform default. Extract a shared
ResolveCallingConvention helper (function convention, falling back to the
platform default) and use it in all three so argument and return-value
handling stay consistent.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Addresses review feedback: Symbol::GetShortName already returns a
std::string, so the explicit std::string(...) construction is redundant.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Addresses review feedback: use std::optional<std::string> instead of an
empty-string sentinel to signal that a call target name could not be
resolved, and update the callers accordingly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>